Skip to content

Provide an opt-in zero-copy response body view - #2322

Open
pavel-ptashyts wants to merge 4 commits into
AsyncHttpClient:mainfrom
maygemdev:perf/zero-copy-response-body-view
Open

Provide an opt-in zero-copy response body view#2322
pavel-ptashyts wants to merge 4 commits into
AsyncHttpClient:mainfrom
maygemdev:perf/zero-copy-response-body-view

Conversation

@pavel-ptashyts

Copy link
Copy Markdown
Contributor

Summary

  • add Response#getResponseBodyAsBytesView() as a compatible default method
    with an explicit possibly-shared, read-only contract
  • return the existing byte array for a single eager Netty body part
  • use the body part accessor for lazy/direct/sliced buffers, avoiding a second
    aggregate copy without exposing ByteBuf storage
  • preserve defensive-copy behavior for getResponseBodyAsBytes(),
    getResponseBodyAsByteBuffer(), and getResponseBodyAsStream()
  • reuse the new accessor for string decoding so the fast path from Decode a lone response body part in place #2303 stays
    centralized

Closes #2321.

Compatibility and safety

The default implementation delegates to getResponseBodyAsBytes(), so existing
third-party Response implementations keep their current behavior. Netty uses
the optimized path only for exactly one body part. Empty and multipart bodies
continue through the existing aggregation path.

Tests cover eager identity and repeated access, defensive-copy isolation,
heap and direct lazy slices, reader/writer indices, reference counts, empty
bodies, multipart ordering and split UTF-8 characters, stream isolation, and
default-method delegation.

Verification

  • mvnw.cmd -B -ntp -pl client -Dtest=NettyAsyncResponseTest test
  • mvnw.cmd clean verify
    • Amazon Corretto 11.0.32.1
    • 1,489 tests; 0 failures; 0 errors; 26 skipped
    • Error Prone, NullAway, Javadocs, GPG signing, and Revapi passed

Allocation benchmark

JMH 1.37, Corretto 11.0.32.1, one fork, three 500 ms warmups, five
500 ms measurements, gc profiler. Values are allocated bytes per operation,
rounded to whole bytes; zero means below the profiler's resolution.

Body shape getResponseBodyAsBytes() getResponseBodyAsBytesView()
Empty 16 B/op 16 B/op
1 eager part, 512 B 576 B/op 0 B/op
1 eager part, 4 KiB 4,160 B/op 0 B/op
1 eager part, 16 KiB 16,448 B/op 0 B/op
1 eager part, 128 KiB 131,136 B/op 0 B/op
1 lazy direct part, 4 KiB 8,272 B/op 4,112 B/op
1 lazy direct part, 16 KiB 32,848 B/op 16,400 B/op
2 eager parts, 4 KiB each 8,256 B/op 8,208 B/op
8 eager parts, 4 KiB each 32,832 B/op 32,832 B/op

The eager single-part view measured about 2.3 ns/op regardless of body size,
versus 322 ns/op for 4 KiB and 11.0 us/op for 128 KiB defensive copies. Lazy
single-part allocation is halved as expected. Multipart bodies retain the
payload-sized aggregation allocation.

Codex on behalf of Pavel Ptashyts

pavel-ptashyts and others added 2 commits August 31, 2026 12:28
Byte-array consumers currently pay for an aggregate copy even when a
response has a single body part. Add an explicit read-only view accessor
so callers can opt into sharing while the existing accessor retains its
defensive-copy contract.

Reuse the view for string decoding and cover eager, lazy, multipart, and
third-party Response implementations.

Refs AsyncHttpClient#2321

Codex on behalf of Pavel Ptashyts

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Mockito 4 cannot invoke interface default methods through
CALLS_REAL_METHODS on JDK 21 and newer. Invoke the Response default
method explicitly so delegation remains covered on every supported JDK.

Refs AsyncHttpClient#2321

Codex on behalf of Pavel Ptashyts

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Comment thread client/src/main/java/org/asynchttpclient/netty/NettyResponse.java Outdated
Comment thread client/src/main/java/org/asynchttpclient/netty/NettyResponse.java
Comment thread client/src/main/java/org/asynchttpclient/netty/NettyResponse.java Outdated
Comment thread client/src/main/java/org/asynchttpclient/netty/NettyResponse.java
Comment thread client/src/main/java/org/asynchttpclient/Response.java Outdated
Comment thread client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java Outdated
Comment thread client/src/test/java/org/asynchttpclient/netty/NettyAsyncResponseTest.java Outdated
Comment thread client/src/main/java/org/asynchttpclient/netty/NettyResponse.java
pavel-ptashyts and others added 2 commits September 6, 2026 09:13
Review feedback on the response body view.

The contract was written as though the sharing were the response's to
give. It is not. Whether a lone body part exists to share from is a fact
about how the body arrived - how the origin chunked it, whether a proxy
re-chunked it, whether it was compressed - so the same body from the
same server can be shared on one response and copied on the next, and
the caller sees no difference. And where an array is shared it is not
the response's alone: it is reachable from the part handed to
onBodyPartReceived, it is what a TransferCompletionHandler gives each
TransferListener, and it is what getResponseBodyAsByteBuf wraps. A write
through any of those changes what this returns, and a write through this
changes what they see.

None of that can be designed away while the method returns a byte array,
so the javadoc says it, and says the identity between calls is not
guaranteed either. It no longer promises anything on behalf of
getResponseBodyAsBytes, which is an interface method whose own contract
guarantees no independent array. It records that leaving the default in
place while implementing getResponseBodyAsBytes in terms of this method
makes the two call each other, which is the first thing an implementor
would otherwise write.

getResponseBody(Charset) goes back through the private helper rather
than the new public method, so overriding the view cannot change what a
response's text says as well; the helper keeps the note about multi-byte
characters straddling a part boundary, which is the only place that
reason is written down, and now says that the array does leave the
client rather than claiming it does not.

An empty body returns a shared empty array rather than walking the
aggregating path to allocate one and a buffer to wrap it. Nothing can be
written through a zero-length array.

The tests asserted identity where they meant content, so an
implementation that shared a corrupted array satisfied them, and one
took its expectation from the array it had handed to the part, which
would have stopped being an oracle the moment the part stopped copying.
The lazy coverage this branch added is dropped: a Response built from
lazy parts holds buffers at refCnt 0 on a real request, since
channelRead releases in a finally and LazyResponseBodyPart never
retains, and a test that keeps one alive by hand signs off on a mode
that does not work. That branch is still covered through
getResponseBody(Charset), which shares the helper.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pavel-ptashyts

Copy link
Copy Markdown
Contributor Author

Round 1 addressed. Two of the comments are about the contract rather than the code, and they are right that neither can be designed away while the method returns a byte[]: whether a lone part exists to share from is decided by the chunking, and where an array is shared it is also reachable from onBodyPartReceived, from a TransferCompletionHandler's listeners and from getResponseBodyAsByteBuf(). So the javadoc states both instead of glossing them, and states that array identity between calls is not guaranteed either.

That leaves an accessor whose honest description is "will not copy when it does not have to, and will not tell you when that is". If that is not worth an interface method, I would rather you said so than have me argue it - it closes cleanly, and getResponseBodyAsByteBuf() already covers the multi-part case zero-copy for anyone who can take a buffer.

The rest:

  • getResponseBody(Charset) goes back through the private helper, so overriding the view cannot change a response's text as well. The helper keeps Decode a lone response body part in place #2303's note about multi-byte characters straddling a part boundary, and now says the array does leave the client rather than claiming it does not.
  • The javadoc no longer promises anything on behalf of getResponseBodyAsBytes(), and records the recursion an implementor would otherwise write - narrowed to the case that actually recurses, which is leaving the default in place.
  • @implSpec is a paragraph rather than a tag: it is not configured for the javadoc plugin here and the build fails on unknown tag. Offered separately.
  • Empty bodies short-circuit to a shared empty array.
  • Tests: content assertions where there were only identity ones, expected.clone() so the oracle is not the fixture, and the LAZY coverage dropped rather than signing off on a mode that cannot occur on a real request.

./mvnw clean verify green. JDK 17 locally, no 11 on this machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide an opt-in zero-copy response body byte-array view

2 participants